-
Notifications
You must be signed in to change notification settings - Fork 1.4k
io: Add support for long range references in TBufferFile #21194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: Arlesienne
Are you sure you want to change the base?
io: Add support for long range references in TBufferFile #21194
Conversation
pcanal
commented
Feb 8, 2026
For now, the testing of the long range references is disabled.
References for class names and objects are stored in 64bits whenever they are located past 1GB. This is the 'only' option as there are no control bits left to distinguish between a 32bits and a 64bits reference.
Reference in the longRange section (>1GB) are now tagged so that we can differentiate nullptr (stored as only 32 bits) from short reference stored in the longRange section (i.e. those are stored in 64 bits but have only zeros in the high bit).
Test Results 22 files + 1 22 suites +1 3d 17h 9m 22s ⏱️ + 4h 45m 44s For more details on these failures, see this check. Results for commit 52b3714. ± Comparison against base commit 588b29e. |
| virtual void PushDataCache(TVirtualArray *); | ||
|
|
||
| virtual TClass *ReadClass(const TClass *cl = nullptr, UInt_t *objTag = nullptr) = 0; | ||
| virtual TClass *ReadClass(const TClass *cl = nullptr, ULong64_t *objTag = nullptr) = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could keep the old signature, forward to the new one and throw if the returned objTag is too large.
| @@ -1375,7 +1375,7 @@ TString *TString::ReadString(TBuffer &b, const TClass *clReq) | |||
| // Before reading object save start position | |||
| UInt_t startpos = UInt_t(b.Length()); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not subject to this PR but should also be 64bit. Same issue for the write counterpart.
| @@ -47,7 +47,7 @@ TArray *TArray::ReadArray(TBuffer &b, const TClass *clReq) | |||
| // Before reading object save start position | |||
| UInt_t startpos = UInt_t(b.Length()); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be 64bit. Same issue for the write counterpart.
| @@ -105,7 +105,7 @@ void TConvertClonesArrayToProxy::operator()(TBuffer &b, void *pmember, Int_t siz | |||
| UInt_t startpos = b.Length(); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
64bit? To be checked for writing.
|
|
||
| constexpr ULong64_t kMaxLongRange = 0x0FFFFFFFFFFFFFFE; // We reserve the 4 highest bits for flags, currently only 2 are in use. | ||
| constexpr ULong64_t kLongRangeClassMask = 0x8000000000000000; // OR the class index with this | ||
| // constexpr ULong64_t kLongRangeByteCountMask = 0x4000000000000000; // OR the byte count with this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be useful to add a comment why this is commented. Because it conflicts with the regular kByteCountMask?
| // This is needed so that the reader can distinguish between references, | ||
| // bytecounts, and new class definitions. | ||
| ULong64_t objIdx = static_cast<ULong64_t>(idx); | ||
| // FIXME: verify that objIdx is guaranteed to fit in 60-bits, i.e. objIdx <= kMaxLongRange |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment can be removed, I think
| // This is needed so that the reader can distinguish between references, | ||
| // bytecounts, and new class definitions. | ||
| ULong64_t clIdx = static_cast<ULong64_t>(idx); | ||
| // FIXME: verify that clIdx is guaranteed to fit in 60-bits, i.e. clIdx <= kMaxLongRange |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed?